Skip to content

fix(spreadsheet): prevent duplicate equipment fetch#3830

Merged
TheMaskedTurtle merged 5 commits intomainfrom
jorism/fix/spreadsheet-multi-fetches
Mar 26, 2026
Merged

fix(spreadsheet): prevent duplicate equipment fetch#3830
TheMaskedTurtle merged 5 commits intomainfrom
jorism/fix/spreadsheet-multi-fetches

Conversation

@TheMaskedTurtle
Copy link
Copy Markdown
Contributor

@TheMaskedTurtle TheMaskedTurtle commented Mar 24, 2026

PR Summary

As fetching elements for spreadsheet is heavy on resource we want to limit them as much as possible. We noticed some cases of fetch duplication that were not necessary :

  • when several spreadsheet are of the same type we want to fetch this type only once in automatic re-fetch
  • when tab is active we don't want to fetch if automatic re-fetch is already doing it

So this PR fixes these two cases.

@TheMaskedTurtle
fix(spreadsheet): prevent duplicate equipment fetch
474f929 
- add isFetching check
- add a set on equipment types

Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>
@TheMaskedTurtle
chore: add comment
807d7a9 
Signed-off-by: Joris Mancini <joris.mancini_externe@rte-france.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 24, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 302a94b4-af8d-444a-8366-918ecd51fd35

📥 Commits

Reviewing files that changed from the base of the PR and between a40f0f4 and 807d7a9.

📒 Files selected for processing (2)
  • src/components/spreadsheet-view/hooks/use-spreadsheet-equipments.ts
  • src/components/spreadsheet-view/spreadsheet/spreadsheet-content/spreadsheet-content.tsx
📝 Walkthrough

Walkthrough

Two changes prevent duplicate equipment data fetching: the hook now deduplicates spreadsheet equipment types using a Set, and the component adds an isFetching condition to prevent redundant fetches when the hook is already re-fetching data.

Changes

Cohort / File(s) Summary
Type Deduplication in Hook
src/components/spreadsheet-view/hooks/use-spreadsheet-equipments.ts		 Modified applyToAllTypes to iterate over a Set of tablesDefinitions[*].type values instead of a potentially duplicated array, ensuring each equipment type is processed at most once per invocation.
Fetch Guard in Component
src/components/spreadsheet-view/spreadsheet/spreadsheet-content/spreadsheet-content.tsx		 Added !equipments.isFetching condition to tab-open fetch logic to prevent concurrent fetch calls when the hook is already performing an automatic re-fetch; updated useEffect dependencies to include equipments.isFetching.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing duplicate equipment fetches in the spreadsheet component.
Description check ✅ Passed The description is directly related to the changeset, explaining the rationale for preventing duplicate fetches and detailing the two specific cases addressed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 26, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sBouzols
sBouzols approved these changes Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sBouzols sBouzols left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review OK
Tests OK
Console warning check OK

@dbraquart dbraquart self-requested a review March 26, 2026 11:20
dbraquart
dbraquart approved these changes Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@dbraquart dbraquart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code: ok
tests: ok

  • the 2 error test cases are fixed
  • non-reg ok

@TheMaskedTurtle TheMaskedTurtle merged commit 3b5c546 into main Mar 26, 2026
6 checks passed
@TheMaskedTurtle TheMaskedTurtle deleted the jorism/fix/spreadsheet-multi-fetches branch March 26, 2026 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sBouzols sBouzols sBouzols approved these changes

@dbraquart dbraquart dbraquart approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TheMaskedTurtle @sBouzols @dbraquart